Skip to content

fix: bin/run error handling path for oclif v4#830

Merged
dthampy merged 1 commit into
masterfrom
fix-oclif-error-handling
Jun 3, 2026
Merged

fix: bin/run error handling path for oclif v4#830
dthampy merged 1 commit into
masterfrom
fix-oclif-error-handling

Conversation

@dthampy
Copy link
Copy Markdown
Contributor

@dthampy dthampy commented Jun 2, 2026

Description

@oclif/core/handle and @oclif/core/flush expose handle and flush as named exports from their subpath modules. The previous code was passing the module object directly into .then() / .catch(), which means the intended handlers were not actually being called correctly.
As a result, errors could escape to Node’s default uncaught exception printer instead of going through oclif’s normal error renderer.

The fix explicitly destructures handle and flush and passes the actual functions into the promise chain
This keeps CLI error handling consistent and avoids leaking raw stack traces in cases where oclif should render a cleaner user-facing error.

Related Issue

https://jira.corp.adobe.com/browse/ACNA-4659

Closes #829

Motivation and Context

aio currently shows raw Node.js stack traces for CLI errors instead of oclif’s clean error output. The root cause is that bin/run passes the @oclif/core/handle and @oclif/core/flush module objects instead of the actual named functions exported by @oclif/core v4.

Because of that, error handling is skipped and user-facing validation/runtime errors are displayed as noisy stack traces. This PR fixes the wiring by passing the actual handle and flush functions, restoring clean CLI error rendering and making command failures easier to understand.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dthampy dthampy requested review from pru55e11 and purplecabbage June 2, 2026 23:50
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 PR Reviewer

The fix correctly addresses a real bug where module objects were being passed instead of functions to .then() and .catch(), which would silently drop error handling. The e2e test is well-structured and validates the regression. The changes are minimal, clear, and correct.

LGTM! This PR looks good to merge.


💡 How to re-trigger

Comment /review or /pr-reviewer on this PR

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@pru55e11 pru55e11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit before merge: the title uses feat: but this is a bug fix — with our np/semantic-release flow feat: cuts a minor version when it should be a patch. Can we retitle to fix: bin/run error handling path for oclif v4? (Squash-merge title matters since that's what lands on the commit.)

@dthampy
Copy link
Copy Markdown
Contributor Author

dthampy commented Jun 3, 2026

One nit before merge: the title uses feat: but this is a bug fix — with our np/semantic-release flow feat: cuts a minor version when it should be a patch. Can we retitle to fix: bin/run error handling path for oclif v4? (Squash-merge title matters since that's what lands on the commit.)

yes we can retitle it to fix. But just an FYI, when we run np --no-publish, it doesn't automatically cut a minor version just because the commit starts with feat:
It presents the semver options and we manually pick patch vs minor based on whether the change is a bug fix or an actual feature

@dthampy dthampy changed the title feat: fix bin/run error handling path for oclif v4 fix: fix bin/run error handling path for oclif v4 Jun 3, 2026
@dthampy dthampy changed the title fix: fix bin/run error handling path for oclif v4 fix: bin/run error handling path for oclif v4 Jun 3, 2026
@dthampy dthampy merged commit 53c26e5 into master Jun 3, 2026
11 checks passed
@dthampy dthampy deleted the fix-oclif-error-handling branch June 3, 2026 14:24
@dthampy dthampy mentioned this pull request Jun 3, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All CLI errors print a raw Node stack trace — bin/run passes the @oclif/core/handle module object (not .handle) to .catch()

2 participants